Skip to content

fix(factory): dispatch after Slack triage clarification#34

Merged
khaliqgant merged 1 commit into
mainfrom
fix/slack-triage-escalation-replies
Jun 22, 2026
Merged

fix(factory): dispatch after Slack triage clarification#34
khaliqgant merged 1 commit into
mainfrom
fix/slack-triage-escalation-replies

Conversation

@miyaontherelay

Copy link
Copy Markdown
Contributor

Summary

  • stop treating expected triage escalations as generic factory errors, so logs no longer show opaque [factory] error {} for this path
  • make triage escalation questions route-aware instead of generic: unknown repo asks for repo(s), thin routed issues ask for expected behavior/constraints/acceptance tests
  • when a human replies to a pre-dispatch triage escalation thread, append that Slack answer as clarification context, re-run triage, and dispatch or queue if the issue is now actionable

Why

AR-287 escalated before dispatch, so there was no implementer alive to receive the Slack answer. Existing Slack reply routing only injected replies into already in-flight implementer/babysitter agents. This patch handles the pre-dispatch case explicitly.

Local status check

  • running factory PID 47081 had registry /tmp/factory-run/factory-loop-registry.json with agents: []
  • no local process command line contained ar-287

Verification

  • npm run build
  • npm test -- src/orchestrator/factory.test.ts (211 pass)
  • npm test (545 pass)
  • git diff --check

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 94295797-e6e2-42ee-87f1-cfc57a0fe74c

📥 Commits

Reviewing files that changed from the base of the PR and between ebdecf9 and dfcee4c.

📒 Files selected for processing (2)
  • src/orchestrator/factory.test.ts
  • src/orchestrator/factory.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/orchestrator/factory.test.ts
  • src/orchestrator/factory.ts

📝 Walkthrough

Walkthrough

FactoryLoop triage escalation handling is refactored to record escalations via a new #recordTriageEscalation helper instead of emitting errors. Slack answer routing gains a new path that detects triage-escalation watch records and, via #handleTriageEscalationSlackAnswer, clarifies the issue description and re-dispatches. Three new pure helpers (triageEscalationQuestion, isTriageEscalationWatchRecord, issueWithSlackClarification) support the flow. Tests add a SlackClarifiedTriage fixture and a new end-to-end escalation-to-dispatch scenario.

Changes

Slack Triage Escalation Retry & Dispatch

Layer / File(s) Summary
Triage escalation recording refactor
src/orchestrator/factory.ts
Both dispatch and #handleChange now call #recordTriageEscalation(decision, reason) instead of #error(new Error(...)). The new helper increments the triageEscalations counter and emits a structured warning log entry.
Pure helpers: question text, watch-record detection, description clarification
src/orchestrator/factory.ts
triageEscalationQuestion generates dynamic question text for escalation Slack threads; isTriageEscalationWatchRecord identifies escalation placeholder records; issueWithSlackClarification appends the Slack reply to an issue description for re-triage. Escalation thread payload is updated to call triageEscalationQuestion.
Slack answer routing and re-triage dispatch handler
src/orchestrator/factory.ts
#routeSlackAnswerToImplementers trims/validates reply text and routes triage-escalation Slack answers to #handleTriageEscalationSlackAnswer, which validates issue state, skips in-flight/queued/blocked issues, clarifies the description, re-runs triage, then dispatches or queues the issue.
Test fixtures and end-to-end coverage
src/orchestrator/factory.test.ts
Adds SlackClarifiedTriage engine, updates existing Slack escalation assertions to match new "Reason:"/"Question:" text, and adds an end-to-end test that injects a Slack reply, flushes twice, and asserts implementer/reviewer spawn and Slack answer-dispatch counter updates.

Sequence Diagram(s)

sequenceDiagram
  participant Client as FactoryLoop
  participant Triage as TriageEngine
  participant Slack as SlackClient
  participant Handler as `#handleTriageEscalationSlackAnswer`

  Client->>Triage: triage(issue)
  Triage-->>Client: low-confidence / thin decision
  Client->>Client: `#recordTriageEscalation`(decision, reason)
  Client->>Slack: escalateTriageToSlack(issue, triageEscalationQuestion(decision))
  Slack-->>Client: watch record stored

  Note over Slack,Client: Human replies on Slack thread

  Slack->>Client: `#routeSlackAnswerToImplementers`(record, replyText)
  Client->>Client: isTriageEscalationWatchRecord(record) → true
  Client->>Handler: `#handleTriageEscalationSlackAnswer`(record, replyText)
  Handler->>Handler: issueWithSlackClarification(issue, replyText)
  Handler->>Triage: triage(clarifiedIssue)
  Triage-->>Handler: high-confidence decision
  Handler->>Client: dispatch or queue clarified issue
  Handler->>Client: update slackTriageAnswerDispatched counter
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 A Slack reply arrives, the triage re-spins,
No more error thrown when the thin issue begins.
Clarify the context, describe what you mean,
The factory re-triages and queues up the scene!
High-confidence hopping, the agents now spawn,
✨ From escalation to dispatch — the rabbit hops on!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(factory): dispatch after Slack triage clarification' accurately summarizes the main change: enabling dispatch to proceed after receiving Slack clarification in triage escalations.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, explaining the motivation, implementation approach, and verification steps for handling pre-dispatch Slack triage clarifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/slack-triage-escalation-replies

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/orchestrator/factory.test.ts`:
- Around line 6843-6845: The test has a timing issue where the assertion
checking factory.status().counters.slackTriageAnswersDispatched executes before
the Slack-triage dispatch operation completes its asynchronous counter update.
Add an additional async drain step before the assertions on lines 6844-6845 to
ensure the dispatch operation fully completes and updates the counters. This
will eliminate the race condition causing the counter to be undefined instead of
1 in CI.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0489059f-03b1-48d7-9d7f-c2cd7667baa2

📥 Commits

Reviewing files that changed from the base of the PR and between 65bc58e and ebdecf9.

📒 Files selected for processing (2)
  • src/orchestrator/factory.test.ts
  • src/orchestrator/factory.ts

Comment thread src/orchestrator/factory.test.ts
@miyaontherelay miyaontherelay force-pushed the fix/slack-triage-escalation-replies branch from ebdecf9 to dfcee4c Compare June 22, 2026 15:19
@khaliqgant khaliqgant merged commit 844036c into main Jun 22, 2026
2 checks passed
@khaliqgant khaliqgant deleted the fix/slack-triage-escalation-replies branch June 22, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants